-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow calling from more than one language #2
base: main
Are you sure you want to change the base?
Conversation
pub struct UniffiForeignPointerCell<T> { | ||
pointers: Mutex<Vec<*mut T>>, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure this could be a better name. Is it still a Cell
given it's holding multiple vtables?
Would this also be better as a Cow<T>
?
return handleMap.get(value) | ||
override fun read(buf: ByteBuffer): CallbackInterface { | ||
val handle = buf.getLong() | ||
assert(buf.getInt() == this.langIndex) { "Callback interface has been called in the wrong language" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't true in all cases: we should be able to call from one language to another via Rust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think there's 2 options here:
- Make it so the Rust scaffolding functions work with foreign handles as well. If Kotlin calls the scaffolding function with a handle from React-native, then Rust will forward the call using the React-native vtable
- Make everything go through vtables.
- Don't export the Rust scaffolding functions directly for a trait interface.
- Instead, export a scaffolding function that inputs a language index and returns the vtable for that language.
- Foreign code uses that to get a vtable whenever they see a handle with a language index they don't recognize. Once it has a vtable, it uses that to make the call. This works the exactly same for Rust and other foreign languages.
The second option seems better for performance and I kind of like not special-casing Rust. I might be a pain to implement though.
if (handleMap.has(handle)) { | ||
return handleMap.get(handle) | ||
} else { | ||
return {{ impl_class_name }}(Pointer(handle)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that this right yet: we should be able to detect here if the trait was implemented in our language, Rust or another foreign language.
fun lowerPointer(pointer: Pointer): RustBuffer.ByValue = | ||
RustBuffer.write(12UL) { bbuf -> | ||
bbuf.putLong(Pointer.nativeValue(pointer)) | ||
bbuf.putInt(langIndex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how we have to handle it with the current handle system. I'm hoping to implement something like mozilla#1823, which would allow us to pack both the pointer and the langIndex
into a 64-bit handle.
This means creating some sort of tagged pointer. The simplest version of that would be using the lower bits for the langIndex. We know that Arc
stores a word ref count, so on a 32-bit system it's aligned to at least 4 bytes. That gives us 2 bits to use, which could support 3 foreign languages plus Rust, do you think that would be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 languages at the same time sounds like it should be enough.
Just for the sake of not boxing ourselves into a corner: I should imagine if we want more than 3 then we'd need to move to 64bits for everything, which also sounds do-able, if absolutely necessary.
This is super interesting and is motivating me to work on the handle code some more. I'd love to do that, then implement this on top of that work. Do you have a timeline for when you want this feature? |
@bendk My timeline for this work is not especially urgent. If the handle work is a prerequisite for this work, then I'm happy to wait. |
Fixes mozilla#2295